feat(rollup-plugin-html): suport transform on input html#3003
feat(rollup-plugin-html): suport transform on input html#3003
Conversation
🦋 Changeset detectedLatest commit: 44fff34 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // run transform functions on input HTML | ||
| const inputTransforms = [...(inputExternalTransformHtmlFns ?? [])]; | ||
| for (const transform of inputTransforms) { | ||
| inputHtml = await transform(inputHtml, { |
There was a problem hiding this comment.
should we add some error handling here?
There was a problem hiding this comment.
good one, in general the exception will be associated with a particular plugin, so locating the problem should be easy enough, but it can be improved always but making it more specific, just maybe logical not to mix up here with this change since the behavior mimics that of the output html transform
or do you want to do it in this PR?
There was a problem hiding this comment.
I think even just a nice readable error message "transform hook threw an error" or something would be fine, can indeed improve in the future.
(not for this PR, but) Typically when I implement plugin systems in my tooling I require them to have a name, e.g.:
const fooPlugin = {
name: 'foo-plugin',
someHook: () => {}
}That way you can throw really helpful error messages that include the name of the plugin it threw on
What I did
GOOD TO MENTION: Do not confuse with Vite's "order", their transforms always apply to the HTML before it goes through the pipeline, and the "order" only defines the priority of transform functions. If we need to add smth like this in the future, we gonna use a 3rd param, e.g. "priority", for that, but this PR has nothing to do with that, just my 2 cents.